Add documentation generation functionality #367

iann0036 · 2019-12-12T05:48:01Z

Looking for feedback on functionality to add documentation generation during a cfn generate. Will generate only initial README.md without correct types for now.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

rjlohan · 2019-12-13T16:29:12Z

Tried running this branch on one of my local repos but hit a bug;

Traceback (most recent call last):
  File "/Users/ryanloha/workspaces/cloudformation-cli/src/rpdk/core/cli.py", line 98, in main
    args.command(args)
  File "/Users/ryanloha/workspaces/cloudformation-cli/src/rpdk/core/generate.py", line 16, in generate
    project.generate_docs()
  File "/Users/ryanloha/workspaces/cloudformation-cli/src/rpdk/core/project.py", line 317, in generate_docs
    contents = template.render(type_name=self.type_name, schema=self.schema)
  File "/Users/ryanloha/workspaces/cloudformation-cli/env/lib/python3.7/site-packages/jinja2/asyncsupport.py", line 76, in render
    return original_render(self, *args, **kwargs)
  File "/Users/ryanloha/workspaces/cloudformation-cli/env/lib/python3.7/site-packages/jinja2/environment.py", line 1008, in render
    return self.environment.handle_exception(exc_info, True)
  File "/Users/ryanloha/workspaces/cloudformation-cli/env/lib/python3.7/site-packages/jinja2/environment.py", line 780, in handle_exception
    reraise(exc_type, exc_value, tb)
  File "/Users/ryanloha/workspaces/cloudformation-cli/env/lib/python3.7/site-packages/jinja2/_compat.py", line 37, in reraise
    raise value.with_traceback(tb)
  File "/Users/ryanloha/workspaces/cloudformation-cli/src/rpdk/core/templates/docs-readme.yml", line 63, in top-level template code
    {% if len(identifierptr) == 1 %}
jinja2.exceptions.UndefinedError: 'len' is undefined

This is running against the schema from this PR: aws-cloudformation/aws-cloudformation-resource-providers-cloudformation#4

It's possible that my schema is broken as I haven't finished that, but at the least we probably want to be more tolerant to bad schemas in this doc generation. I think it's a valid schema though, as validation should happen as part of cfn generate before it gets to this new doc generation path.

iann0036 · 2019-12-14T04:49:53Z

TIL you can use composite keys for the primaryIdentifier...will cater for that.

Working on the sub-property pages and type detection now.

iann0036 · 2019-12-14T10:37:50Z

@rjlohan Docs generation is now complete. Seeking full review.

tobywf

nice! will take a look fully next week. there's a bunch of stuff around using pathlib that would be great to do. although our CI doesn't build Windows yet, would be great to keep it OS agnostic.

src/rpdk/core/project.py

tests/test_project.py

iann0036 · 2019-12-16T04:42:51Z

Thanks @tobywf!

Is there a cleaner way to do this? Looked at JsonSchemaFlattener and it seemed close but not quite what I needed to resolve $ref's.

tobywf · 2019-12-16T07:59:53Z

if i'm reading it right, i think John did something similar in #315 with RefResolver from jsonschema:

cloudformation-cli/src/rpdk/core/contract/resource_generator.py

Lines 75 to 76 in 06d5fa9

    
           def resolve_ref(self, schema): 
        
               return self.resolver.resolve(schema["$ref"])[1]

maybe can use a similar approach? (it isn't ideal since it's an internal class - but we haven't gotten around to write our own, and something already uses it, so fine for now.)

iann0036 · 2019-12-16T09:28:51Z

if i'm reading it right, i think John did something similar in #315 with RefResolver from jsonschema:

That RefResolver class was perfect! Replaced in 1a31248.

tobywf

I do love this feature, but one of my concerns is this feature is opted in by default. I's complex, and simply doesn't work reliably yet when running against real schemas, e.g.:

  File "aws-cloudformation-rpdk/src/rpdk/core/templates/docs-readme.yml", line 55, in top-level template code
    _Allowed Values_: <code>{{ "</code> | <code>".join(prop.allowedvalues) }}</code>
TypeError: sequence item 0: expected str instance, int found

I also get weird doubling in the sub-property page titles "AWS::Foo::Bar Spam Spam Eggs Eggs" (again, real schema). I'll spend some more time and see if I can debug some of these issues. Do you mind me pushing new commits to this PR, or would you prefer I branch off?

tests/test_project.py

iann0036 · 2020-01-22T21:53:26Z

@tobywf Fine for you to make direct edits.

Also happy for the docs generation to be opt-in (cfn generate flag?).

From memory, when generating sub-property pages I had it generate AWS::Foo::Bar Spam Eggs format (and not just the last element) in order to avoid having two pages of the exact same title - I believe the AWS:: types always have explicit names for these definitions, but this may not be the case in other schemas. Agree the double up is a bug though.

tobywf · 2020-01-22T22:48:04Z

I'm just going to run it against a bunch of schemas and see what happens. If it works reliably, no need to make it opt-in.

tobywf · 2020-01-23T00:03:27Z

Okay, fixed the issue with different data types, debugging the duplication. It happens with arrays:

propname: Foos, proppath: ['Foos'] | prop: {'type': 'array', 'items': {'$ref': '#/definitions/Foo'}}

elif prop["type"] == "array":
    prop["arrayitems"] = self._set_docs_properties(
        propname, prop["items"], proppath
    )

propname: Foos, proppath: ['Foos', 'Foos'] | prop: {'$ref': '#/definitions/Foo'}

An idea is using "Items" as propname? For AccessAnalyzer, that results in:

archiverules-items-filter-items.md
archiverules-items.md
tags-items.md

But not doing the duplication is what I've gone with for now.

iann0036 · 2020-01-23T00:11:25Z

If you appended -items- to those you'd have to do similar for actual properties (e.g. someobj-properties-key.md) so there's no issue when the actual key Items: exists.

* Avoid `.join()` in templates; it expects strings only * Use existing functions to decode JSON pointers * Add `.md` to Jinja autoescape settings to prevent HTML injection from e.g. schema descriptions * Also rename templates to correct file-ending of resulting files * Avoid modifying dictionaries in a loop where possible * Check for nested primaryIdentifier * Support nested additionalIdentifiers (but not compound ones) * Make proppath a tuple, so it is immutable * Avoid modifying proppath for type "array" to avoid duplicate propname in proppath * Liberal use of f-strings

tobywf · 2020-01-23T01:21:36Z

awesome, I tried it against a bunch of schemas, and it works great. got a bit carried away, we have functions to parse the JSON pointers. i think we'll try and get this out ASAP in the next release.

src/rpdk/core/project.py

tobywf · 2020-01-24T22:04:47Z

Found another issue: read-only properties should really be excluded, since they can't be specified.

PatMyron · 2020-02-24T01:17:18Z

It could be interesting to include regional availability information here eventually as well

Calling the CloudFormation registry directly across multiple partitions with valid credentials doesn't seem practical, but registry type regional availability information might be obtainable through another data source

…autodoc # Conflicts: # tests/test_project.py

iann0036 · 2020-03-18T10:16:33Z

True-up'd with master.

rjlohan

Nice one 🥇

wbingli · 2020-03-21T00:25:25Z

Nice work!

Tested with one resource, docs looks great: https://gist.github.com/wbingli/f725de6f7950067b250f8f35a8c7ec09

PatMyron · 2020-03-21T03:08:46Z

src/rpdk/core/templates/docs-readme.md

+{% if prop.createonly %}
+
+_Update requires_: [Replacement](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-replacement)
+{% else %}
+
+_Update requires_: [No interruption](https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-no-interrupt)
+{% endif %}


If we're generating documentation, it's probably worth noting nuance for something as important as potential resource replacement. For example, a version property might be upgradable in-place but not downgradable in-place, so neither of these definitions would be entirely accurate. My assumption would be while schema definitions like required and createOnlyProperties guarantee a property is required or requires resource replacement to update respectively, other properties not specified as required or createOnlyProperties may in fact be required or cause resource replacement under some circumstances. Immutability is important enough that its nuance should probably be captured in the resource schema itself in the future

Real world example courtesy of @iann0036:
Updating AWS::EC2::VPC.InstanceTenancy requires no replacement only if you are updating its value from "dedicated" to "default". Updating AWS::EC2::VPC.InstanceTenancy from "default" to "dedicated" requires replacement

PatMyron · 2020-08-05T19:56:47Z

src/rpdk/core/project.py

+BASIC_TYPE_MAPPINGS = {
+    "string": "String",
+    "number": "Double",
+    "integer": "Double",


This should be Integer, right?

Suggested change

"integer": "Double",

"integer": "Integer",

PatMyron · 2020-09-22T23:32:42Z

src/rpdk/core/project.py

+    @staticmethod
+    def _get_docs_gettable_atts(docs_schema):
+        def _get_property_description(prop):
+            path = fragment_decode(prop, prefix="")


#367 (comment) aws-cloudformation/cloudformation-resource-schema#121 https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-some-interrupt

* generate_docs() for conditionalCreateOnlyProperties #367 (comment) aws-cloudformation/cloudformation-resource-schema#121 https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-some-interrupt * test coverage

* generate_docs() for conditionalCreateOnlyProperties aws-cloudformation/cloudformation-cli#367 (comment) aws-cloudformation/cloudformation-resource-schema#121 https://docs.aws.amazon.com/AWSCloudFormation/latest/UserGuide/using-cfn-updating-stacks-update-behaviors.html#update-some-interrupt * test coverage

iann0036 added 2 commits December 12, 2019 11:50

Add initial documentation generation functionality

14ba506

Add tests for docs generation

586677b

Add subproperty pages to docs generation

1dedbd8

iann0036 marked this pull request as ready for review December 14, 2019 10:36

tobywf reviewed Dec 16, 2019

View reviewed changes

iann0036 added 2 commits December 16, 2019 15:26

Adjustments for review comments #367

e29f95e

Replace os lib with pathlib funcs #367

2c4e92c

iann0036 added 2 commits December 16, 2019 20:17

Use previously built RefResolver in docs generation #367

1a31248

Fix missed pathlib change for docs generation #367

bafad87

tobywf reviewed Jan 22, 2020

View reviewed changes

tests/test_project.py Outdated Show resolved Hide resolved

Merge remote-tracking branch 'upstream/master' into feature/autodoc

a8babbc

tobywf approved these changes Jan 23, 2020

View reviewed changes

tobywf requested a review from johnttompkins January 23, 2020 01:22

johnttompkins reviewed Jan 23, 2020

View reviewed changes

src/rpdk/core/project.py Show resolved Hide resolved

src/rpdk/core/project.py Outdated Show resolved Hide resolved

tobywf added 2 commits January 24, 2020 13:41

Merge remote-tracking branch 'upstream/master' into feature/autodoc

e68d669

Use read-only properties for GetAtt

7ab516c

iann0036 added 2 commits March 18, 2020 21:05

Merge commit 'e1999bcba76692ed614237d3478f9fff6bf905b2' into feature/…

71218c6

…autodoc # Conflicts: # tests/test_project.py

Update modified function call for write_settings

0ad18ed

rjlohan approved these changes Mar 18, 2020

View reviewed changes

wbingli self-requested a review March 21, 2020 00:26

wbingli approved these changes Mar 21, 2020

View reviewed changes

wbingli merged commit c85cd03 into aws-cloudformation:master Mar 21, 2020

PatMyron reviewed Mar 21, 2020

View reviewed changes

iann0036 mentioned this pull request Mar 21, 2020

Documentation generation to ignore read-only properties and remove autoescaping Jinja #409

Merged

PatMyron mentioned this pull request Apr 14, 2020

removing generated documentation aws-cloudformation/aws-cloudformation-resource-providers-sns#6

Merged

PatMyron reviewed Aug 5, 2020

View reviewed changes

PatMyron reviewed Sep 22, 2020

View reviewed changes

PatMyron mentioned this pull request Sep 22, 2020

Unhandled exception in generate_docs() #581

Closed

PatMyron mentioned this pull request Dec 23, 2020

Updating ABAC schema to have AccessControlAttributes on top level aws-cloudformation/aws-cloudformation-resource-providers-sso#12

Merged

PatMyron mentioned this pull request Jan 13, 2021

correcting update behavior descriptions awsdocs/aws-cloudformation-user-guide#904

Merged

PatMyron mentioned this pull request Apr 26, 2021

generate_docs() for conditionalCreateOnlyProperties #740

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add documentation generation functionality #367

Add documentation generation functionality #367

iann0036 commented Dec 12, 2019

rjlohan commented Dec 13, 2019

iann0036 commented Dec 14, 2019

iann0036 commented Dec 14, 2019

tobywf left a comment

iann0036 commented Dec 16, 2019

tobywf commented Dec 16, 2019 •

edited

Loading

iann0036 commented Dec 16, 2019

tobywf left a comment

iann0036 commented Jan 22, 2020

tobywf commented Jan 22, 2020

tobywf commented Jan 23, 2020

iann0036 commented Jan 23, 2020

tobywf commented Jan 23, 2020

tobywf commented Jan 24, 2020

PatMyron commented Feb 24, 2020 •

edited

Loading

iann0036 commented Mar 18, 2020

rjlohan left a comment

wbingli commented Mar 21, 2020 •

edited

Loading

PatMyron Mar 21, 2020

PatMyron Mar 21, 2020 •

edited

Loading

PatMyron Aug 5, 2020 •

edited

Loading

PatMyron Sep 22, 2020

Add documentation generation functionality #367

Add documentation generation functionality #367

Conversation

iann0036 commented Dec 12, 2019

rjlohan commented Dec 13, 2019

iann0036 commented Dec 14, 2019

iann0036 commented Dec 14, 2019

tobywf left a comment

Choose a reason for hiding this comment

iann0036 commented Dec 16, 2019

tobywf commented Dec 16, 2019 • edited Loading

iann0036 commented Dec 16, 2019

tobywf left a comment

Choose a reason for hiding this comment

iann0036 commented Jan 22, 2020

tobywf commented Jan 22, 2020

tobywf commented Jan 23, 2020

iann0036 commented Jan 23, 2020

tobywf commented Jan 23, 2020

tobywf commented Jan 24, 2020

PatMyron commented Feb 24, 2020 • edited Loading

iann0036 commented Mar 18, 2020

rjlohan left a comment

Choose a reason for hiding this comment

wbingli commented Mar 21, 2020 • edited Loading

PatMyron Mar 21, 2020

Choose a reason for hiding this comment

PatMyron Mar 21, 2020 • edited Loading

Choose a reason for hiding this comment

PatMyron Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

PatMyron Sep 22, 2020

Choose a reason for hiding this comment

tobywf commented Dec 16, 2019 •

edited

Loading

PatMyron commented Feb 24, 2020 •

edited

Loading

wbingli commented Mar 21, 2020 •

edited

Loading

PatMyron Mar 21, 2020 •

edited

Loading

PatMyron Aug 5, 2020 •

edited

Loading